Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

agg enable corr covar_pop and covar_samp #147

Merged
merged 2 commits into from
Mar 30, 2023

Conversation

liujiayi771
Copy link

enable corr, covar_pop and covar_samp.

@rui-mo rui-mo merged commit 1d9676b into oap-project:main Mar 30, 2023
zhejiangxiaomai pushed a commit to zhejiangxiaomai/velox that referenced this pull request Mar 30, 2023
* Enable corr covar_pop and covar_samp in aggregate.

* Adjust the accuracy to align with Spark.
zhejiangxiaomai pushed a commit to zhejiangxiaomai/velox that referenced this pull request Mar 30, 2023
* Enable corr covar_pop and covar_samp in aggregate.

* Adjust the accuracy to align with Spark.
zhejiangxiaomai pushed a commit to zhejiangxiaomai/velox that referenced this pull request Apr 14, 2023
* Enable corr covar_pop and covar_samp in aggregate.

* Adjust the accuracy to align with Spark.
zhejiangxiaomai pushed a commit to zhejiangxiaomai/velox that referenced this pull request Apr 17, 2023
* Enable corr covar_pop and covar_samp in aggregate.

* Adjust the accuracy to align with Spark.
zhejiangxiaomai pushed a commit to zhejiangxiaomai/velox that referenced this pull request Apr 19, 2023
* Enable corr covar_pop and covar_samp in aggregate.

* Adjust the accuracy to align with Spark.
zhejiangxiaomai pushed a commit to zhejiangxiaomai/velox that referenced this pull request Apr 20, 2023
* Enable corr covar_pop and covar_samp in aggregate.

* Adjust the accuracy to align with Spark.
@zhejiangxiaomai
Copy link
Collaborator

Would you create a pr to upstream this change to velox community when you are free? @liujiayi771

@liujiayi771
Copy link
Author

Would you create a pr to upstream this change to velox community when you are free? @liujiayi771

OK. I will discuss this order of calculation with meta. Because the current modification is to meet the calculation results consistent with spark, but it may lead to inconsistent results with presto, we need to see how to solve it better.

@zhejiangxiaomai
Copy link
Collaborator

Would you create a pr to upstream this change to velox community when you are free? @liujiayi771

OK. I will discuss this order of calculation with meta. Because the current modification is to meet the calculation results consistent with spark, but it may lead to inconsistent results with presto, we need to see how to solve it better.

Thank you very much to explain the root cause.

@liujiayi771
Copy link
Author

Would you create a pr to upstream this change to velox community when you are free? @liujiayi771

OK. I will discuss this order of calculation with meta. Because the current modification is to meet the calculation results consistent with spark, but it may lead to inconsistent results with presto, we need to see how to solve it better.

Thank you very much to explain the root cause.

I will try to fix this on our side without change Velox codes.

@liujiayi771
Copy link
Author

We should import Epsilon Compare for floating-point value.

@rui-mo
Copy link
Collaborator

rui-mo commented May 16, 2023

We should import Epsilon Compare for floating-point value.

Hi @liujiayi771, do you suggest importing it in Gluten CI?

@liujiayi771
Copy link
Author

Hi, @rui-mo , maybe we can allow a certain error for floating-point numbers when comparing gluten results with vanilla spark. This is what velox does when comparing results with duckdb.

@rui-mo
Copy link
Collaborator

rui-mo commented May 17, 2023

Hi, @rui-mo , maybe we can allow a certain error for floating-point numbers when comparing gluten results with vanilla spark. This is what velox does when comparing results with duckdb.

@liujiayi771 That makes sense to me. We can add some precision tolerance when comparing floats.

@liujiayi771 liujiayi771 deleted the covariance branch October 28, 2023 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants